Skip to content

ci: replace global RUST_TEST_THREADS=1 with per-crate --test-threads=1#280

Merged
Brooooooklyn merged 5 commits intomainfrom
claude/reproduce-flaky-failure-RuwlG
Mar 21, 2026
Merged

ci: replace global RUST_TEST_THREADS=1 with per-crate --test-threads=1#280
Brooooooklyn merged 5 commits intomainfrom
claude/reproduce-flaky-failure-RuwlG

Conversation

@branchseer
Copy link
Member

@branchseer branchseer commented Mar 20, 2026

Summary

  • Removes global RUST_TEST_THREADS=1 from the musl CI job
  • Splits cargo test into non-PTY tests (parallel) and PTY tests (--test-threads=1)
  • Non-PTY tests now run in parallel on musl, improving CI speed

Details

On musl, fork() is unsafe when other threads exist — even threads sleeping on a futex leave musl internal state that the child inherits in an inconsistent form, causing SIGSEGV between fork and exec.

A process-wide mutex (PTY_LOCK) cannot fix this because the test harness still spawns multiple threads. The real fix is --test-threads=1 for PTY test binaries only (pty_terminal, pty_terminal_test, vite_task_bin), ensuring a single thread exists during fork.

Test plan

  • Verify musl CI job passes
  • Run 10 musl test re-runs to confirm stability
  • Verify all other CI jobs still pass

https://claude.ai/code/session_011H8UR3gS6hoyQAf2x7Dfw8

@branchseer branchseer force-pushed the claude/reproduce-flaky-failure-RuwlG branch 3 times, most recently from e990dd9 to 9bf941d Compare March 20, 2026 13:16
@branchseer branchseer changed the title fix: remove RUST_TEST_THREADS=1 for musl by holding PTY_LOCK for entire session ci: replace global RUST_TEST_THREADS=1 with per-crate --test-threads=1 Mar 20, 2026
Copy link
Member

Brooooooklyn commented Mar 21, 2026

Merge activity

  • Mar 21, 5:33 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 21, 5:33 AM UTC: Graphite rebased this pull request as part of a merge.
  • Mar 21, 5:45 AM UTC: @Brooooooklyn merged this pull request with Graphite.

claude added 5 commits March 21, 2026 05:33
On musl, fork() is unsafe when other threads exist — even threads
sleeping on a futex leave musl internal state that the child inherits
in an inconsistent form, causing SIGSEGV between fork and exec.

A process-wide mutex (PTY_LOCK) cannot fix this because the test
harness still spawns multiple threads (one per test). The real fix
is --test-threads=1 for PTY test binaries, which ensures only one
thread exists during fork.

Split `cargo test` into:
- Non-PTY tests: run in parallel (default thread count)
- PTY tests (pty_terminal, pty_terminal_test, vite_task_bin):
  run with --test-threads=1

This removes the global RUST_TEST_THREADS=1 that forced ALL tests
to run sequentially, improving CI speed for non-PTY tests.

https://claude.ai/code/session_011H8UR3gS6hoyQAf2x7Dfw8
On musl libc, fork() in a multi-threaded process is unsafe because musl
internal state (locks, allocator metadata) in other threads can be left
inconsistent in the child, causing SIGSEGV/SIGBUS. portable_pty uses
pre_exec() for PTY setup, which forces Rust's Command to use fork()+exec()
instead of posix_spawn().

This commit bypasses portable_pty's spawn_command() on musl and uses
posix_spawn() directly, which musl implements via clone(CLONE_VM|CLONE_VFORK)
— avoiding fork() entirely. PTY setup is handled via spawn attributes and
file actions:
- POSIX_SPAWN_SETSID for session leadership (replaces setsid())
- addopen() on the slave TTY path sets the controlling terminal
- SETSIGDEF/SETSIGMASK for signal cleanup (replaces manual signal reset)

This allows all tests (including PTY tests) to run in parallel on musl,
removing the need for per-crate --test-threads=1.

https://claude.ai/code/session_011H8UR3gS6hoyQAf2x7Dfw8
On musl libc, fork() in a multi-threaded process is unsafe because musl
internal state (locks, allocator metadata) in other threads can be left
inconsistent in the child, causing SIGSEGV/SIGBUS.

cargo-nextest runs each test in its own process (not thread), so fork()
happens from a mostly single-threaded context. Tests still execute in
parallel across processes, maintaining fast CI times without SIGSEGV.

This replaces the per-crate --test-threads=1 workaround which serialized
all tests within each crate.

https://claude.ai/code/session_011H8UR3gS6hoyQAf2x7Dfw8
…o test

cargo-nextest can't discover tests in packages with `harness = false`
(e2e_snapshots, plan_snapshots) because they output custom formats
instead of the standard libtest listing. Split the musl test run:
- nextest for standard-harness tests (parallel, per-process isolation)
- cargo test for custom-harness packages (already run sequentially)

https://claude.ai/code/session_011H8UR3gS6hoyQAf2x7Dfw8
@Brooooooklyn Brooooooklyn force-pushed the claude/reproduce-flaky-failure-RuwlG branch from 15e7473 to c0a3c55 Compare March 21, 2026 05:33
@Brooooooklyn Brooooooklyn merged commit fa42ef9 into main Mar 21, 2026
9 checks passed
@Brooooooklyn Brooooooklyn deleted the claude/reproduce-flaky-failure-RuwlG branch March 21, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants